Skip to content

fix(cli): error messages for utils/update + utils/command + error library migration#1257

Merged
John-David Dalton (jdalton) merged 8 commits intomainfrom
jdd/error-msg-update-command
Apr 24, 2026
Merged

fix(cli): error messages for utils/update + utils/command + error library migration#1257
John-David Dalton (jdalton) merged 8 commits intomainfrom
jdd/error-msg-update-command

Conversation

@jdalton
Copy link
Copy Markdown
Contributor

@jdalton John-David Dalton (jdalton) commented Apr 22, 2026

Summary

Lands the error-library migration + doctrine rewrite together with the 4-ingredient error-message alignment for utils/update/ and utils/command/. This PR supersedes #1261 — all of that PR's content is folded in via merge commit.

What's in this PR

1. Error-message 4-ingredient alignment (original scope of #1257)

  • packages/cli/src/utils/command/registry-core.mts — what/where/saw/fix format, correct offending-middleware index (bugbot #3125070395)
  • packages/cli/src/utils/update/checker.mts — 4-ingredient format, NetworkUtils.fetch(url) in the error text (bugbot #3125070417)
  • packages/cli/src/utils/update/manager.mts — 4-ingredient format
  • Matching unit-test regex updates

2. Error-library migration (from #1261)

  • packages/cli/src/utils/error/errors.mts + display.mts — adopt @socketsecurity/lib/errors helpers (isError, errorMessage, errorStack, isErrnoException) in place of ad-hoc instanceof Error ? … : String(…) patterns
  • Removes the CLI-local isErrnoException implementation; the lib version is authoritative
  • Propagated to: cli-entry.mts, AnalyticsRenderer.mts, AuditLogRenderer.mts, convert-gradle-to-maven.mts, convert-sbt-to-maven.mts, ThreatFeedRenderer.mts, basics/spawn.mts, debug.mts, git/github.mts, git/gitlab-provider.mts, process/os.mts, telemetry/integration.mts, telemetry/service.mts, update/notifier.mts
  • @socketsecurity/lib bump in pnpm-workspace.yaml + pnpm-lock.yaml

3. Doctrine (from #1261)

  • CLAUDE.md Error Messages section rewritten to match socket-repo-template fleet doctrine: four ingredients stay, audience-based length guidance added (library API terse / CLI verbose / programmatic rule-only), baseline rules tightened. Preserves the CLI-specific InputError / AuthError examples.
  • New docs/references/error-messages.md (166 lines): cross-fleet worked examples and anti-patterns so CLAUDE.md stays under the 40 KB ceiling.

Verification (local, on merged branch)

  • pnpm --filter @socketsecurity/cli run type — clean
  • pnpm --filter @socketsecurity/cli run lint — 0 warnings, 0 errors
  • pnpm --filter @socketsecurity/cli run build — succeeds
  • Unit tests: test/unit/utils/update/ + test/unit/utils/command/ + test/unit/utils/error/ + debug.test.mts334 / 334 passed

Risk

Medium. Error-path refactor touches many callers; @socketsecurity/lib bump could shift surfaced error/cause strings and debug/telemetry output subtly. No business-logic changes.

Related PRs (sibling error-message batches)

Supersedes

Test plan

  • CI type-check + lint + build green on the merged branch
  • Affected unit tests pass (334/334)
  • Reviewer spot-check: utils/error/errors.mts API surface unchanged for external callers
  • Reviewer spot-check: CLAUDE.md Error Messages section reads at junior-dev level

…ingredient strategy

Rewrites error messages across packages/cli/src/utils/update/ and
packages/cli/src/utils/command/ to follow the What / Where /
Saw vs. wanted / Fix strategy from CLAUDE.md.

Sources:
- utils/update/checker.mts: 8 messages (URL validation, package name
  / registry URL validation, version-response validation). Each now
  names the function, the received type, and what a valid value
  looks like.
- utils/update/manager.mts: 3 messages (mirror guards for name /
  version / ttl). Still warn-and-return-false, but the text now
  tells the caller exactly which option was wrong.
- utils/command/registry-core.mts: 6 messages (command / alias
  registration conflicts, middleware next() misuse, flag parsing
  failures). Each now names the offending command, flag name, or
  index so debuggers don't need to read source.

Tests updated:
- test/unit/utils/update/checker.test.mts: 6 assertions (switched
  to regex)
- test/unit/utils/update/manager.test.mts: 3 assertions (switched
  to expect.stringContaining)
- test/unit/utils/command/registry-core.test.mts: 5 assertions

All 153 tests in the affected suites pass.

Follows strategy from #1254. Part of the multi-PR series started
by #1255 (commands/) and continued in #1256 (utils/dlx/).
Comment thread packages/cli/src/utils/command/registry-core.mts
Comment thread packages/cli/src/utils/update/checker.mts
Two issues flagged by Cursor bugbot on #1257:

1. (Medium) registry-core.mts middleware error reported the wrong
   offending middleware index. `index` tracks the highest dispatched
   position, not the caller; when dispatch(i) is re-entered after a
   double-next(), the offender held middleware[i - 1]'s next closure.
   Fixed to use `i - 1` with a comment explaining why.

2. (Low) checker.mts error referenced a non-existent
   `UpdateChecker.fetch()` — the object is actually exported as
   `NetworkUtils`. Renamed in both the error and its test regex.

Caught by #1257 bugbot review.
…s doc

Restructure the CLI-specific Error Messages section to match the
updated fleet doctrine from socket-repo-template:

- Keep the four ingredients (What / Where / Saw vs. wanted / Fix).
- Add audience-based length guidance (library API terse / CLI verbose /
  programmatic rule-only). `InputError`/`AuthError` usages are
  verbose-tier.
- Tighten baseline rules to one-liners; drop narrative phrasing.
- Preserve the CLI-specific examples (--pull-request, socket init,
  AuthError) — they earn their keep as real anti-pattern fodder.

Add `docs/references/error-messages.md` with fleet-wide worked examples
so CLAUDE.md stays tight and the rich anti-patterns live once.
Point readers at @socketsecurity/lib/arrays' list-formatting helpers
from CLAUDE.md (one-line rule) and the worked-examples references doc
(new "Formatting lists of values" section).
Fleet-wide migration to the caught-value helpers in
@socketsecurity/lib/errors.

- pnpm-workspace.yaml: catalog bump 5.21.0 → 5.24.0.
- 18 src files under packages/cli/src: replace every
  `e instanceof Error ? e.message : String(e)` and `UNKNOWN_ERROR`
  fallback with `errorMessage(e)`; replace bare `x instanceof Error`
  boolean checks with `isError(x)`; replace
  `e instanceof Error ? e.stack : undefined` with `errorStack(e)`.
- packages/cli/src/utils/error/errors.mts: drop the locally-defined
  `isErrnoException` (which accepted any `code !== undefined`) and
  re-export the library's stricter string-code variant.
- packages/cli/src/commands/manifest/convert-{gradle,sbt}-to-maven.mts:
  rename a local `const errorMessage` → `summary` to free the
  identifier for the imported helper.
- packages/cli/src/utils/telemetry/service.mts: rename two local
  `const errorMessage = …` variables to `errMsg` inside catch
  blocks for the same reason.
- docs/references/error-messages.md: pick up the new "Working with
  caught values" section from socket-repo-template.

Out of scope (intentionally left):
- Exit-code checks on child-process results (`'code' in e` where
  `code` is a number, e.g. display.mts:257). `isErrnoException`
  requires a string code and would wrongly return false.
- The local `getErrorMessage` / `getErrorMessageOr` helpers in
  errors.mts — callers outside this file still use them; a
  broader refactor to the library `errorMessage` is follow-up.

Pre-commit tests skipped (DISABLE_PRECOMMIT_TEST); `pnpm run type`
and `pnpm run lint` pass.
Cursor bugbot flagged the while(currentCause) loop in displayError: it
walks the .cause chain manually but was calling errorMessage() on each
level, which itself walks the entire remaining chain via
messageWithCauses. For A → B → C, that printed "B msg: C msg" at depth
1, then "C msg" at depth 2, showing C's message twice.

Switch to reading `.message` directly (matching the pre-PR behavior
the bot pointed to) so each iteration emits only that level's text.
Fall back to `String(currentCause)` for non-Error nodes in the chain.

Drop the now-unused `errorMessage` import.

Reported on PR #1261.
…ntics

The debugApiResponse test expected errorMessage('String error') to
return the 'Unknown error' sentinel, matching the old local shim's
behavior that treated any non-Error caught value as unusable. The
catalog bump to @socketsecurity/lib 5.24 switched debug.mts to the
upstream errorMessage, which preserves non-empty primitives as-is —
only empty strings, null, undefined, and plain objects coerce to the
sentinel. Assert on 'String error' to pin the current contract.
@jdalton
Copy link
Copy Markdown
Contributor Author

bugbot run

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 952b914. Configure here.

Fold the fleet-doctrine error-message rewrite + @socketsecurity/lib/errors
migration into this branch so the whole error-messages initiative lands
as one PR:

- CLAUDE.md Error Messages section aligned with fleet doctrine
- New docs/references/error-messages.md (worked examples, anti-patterns)
- packages/cli/src/utils/error/{errors,display}.mts + callers migrated
  to @socketsecurity/lib/errors helpers (isError, errorMessage,
  errorStack, isErrnoException). CLI-local isErrnoException removed.
- @socketsecurity/lib bump in pnpm-workspace.yaml

Supersedes #1261.
@jdalton John-David Dalton (jdalton) changed the title fix(cli): align utils/update/ + utils/command/ error messages with 4-ingredient strategy fix(cli): error messages for utils/update + utils/command + error library migration Apr 24, 2026
@socket-security
Copy link
Copy Markdown

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Updatednpm/​@​socketsecurity/​lib@​5.21.0 ⏵ 5.24.0100100100100100

View full report

@socket-security-staging
Copy link
Copy Markdown

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Updatednpm/​@​socketsecurity/​lib@​5.21.0 ⏵ 5.24.088 -11100100100100

View full report

@socket-security-staging
Copy link
Copy Markdown

Warning

Review the following alerts detected in dependencies.

According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.

Action Severity Alert  (click "▶" to expand/collapse)
Warn High
Obfuscated code: npm @socketsecurity/lib is 90.0% likely obfuscated

Confidence: 0.90

Location: Package overview

From: package.jsonnpm/@socketsecurity/lib@5.24.0

ℹ Read more on: This package | This alert | What is obfuscated code?

Next steps: Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support@socket.dev.

Suggestion: Packages should not obfuscate their code. Consider not using packages with obfuscated code.

Mark the package as acceptable risk. To ignore this alert only in this pull request, reply with the comment @SocketSecurity-Staging ignore npm/@socketsecurity/lib@5.24.0. You can also ignore all packages with @SocketSecurity-Staging ignore-all. To ignore an alert for all future pull requests, use Socket's Dashboard to change the triage state of this alert.

View full report

@jdalton John-David Dalton (jdalton) merged commit e272f33 into main Apr 24, 2026
13 checks passed
@jdalton John-David Dalton (jdalton) deleted the jdd/error-msg-update-command branch April 24, 2026 19:50
John-David Dalton (jdalton) added a commit that referenced this pull request Apr 24, 2026
…, terminal) (#1260)

* fix(cli): align utils/ miscellaneous error messages with 4-ingredient strategy

Final PR in the error-message series. Covers everything not already
touched by #1255-#1259: utils/basics, utils/config, utils/fs,
utils/git, utils/npm, utils/promise, utils/terminal, and the flags
module at the root of the CLI tree.

Sources:
- flags.mts: 2 throws (--max-old-space-size, --max-semi-space-size)
  — name the flag, show the offending value, suggest a concrete
  megabyte value.
- utils/config.mts: 1 throw (SOCKET_CLI_CONFIG base64 decode) —
  explains the replacement-character symptom and how to re-encode.
- utils/basics/vfs-extract.mts: 4 throws (SEA VFS extraction for
  Python + security tools) — name the missing paths, the exit
  codes, and point at the "rebuild the SEA binary" fix.
- utils/promise/queue.mts: 1 throw (PromiseQueue concurrency guard)
  — show the offending value and suggest 4/8.
- utils/npm/spec.mts: 1 throw (PURL conversion) — show the input,
  state what a valid npm spec looks like.
- utils/git/operations.mts: 1 throw (git-not-on-PATH) — point at
  install and the local-path env-var override.
- utils/git/gitlab-provider.mts: 2 throws (no token, PR creation
  after retries) — name the token scope, the retry count, the
  repo/head refs.
- utils/fs/path-resolve.mts: 1 throw (npm path-walk iteration cap)
  — name the start path, current directory, and what usually
  causes the cycle (symlinks).
- utils/terminal/iocraft.mts: 1 throw (native-module load failure)
  — show the underlying error and the offending platform/arch
  triple.

Skipped (already informative):
- github-provider.mts pass-through errors (forward inner CResult
  cause/message)
- gitlab-provider.mts try/catch wrappers that call
  formatErrorWithDetail (inner error has context)
- 'process.exit called' sentinel throws in npm/pnpm/yarn/with-
  subcommands paths (test harness re-raise markers, not user-facing)

Tests updated:
- test/unit/utils/promise/queue.test.mts (2 assertions)
- test/unit/utils/npm/spec.test.mts (2 assertions)
- test/unit/utils/git/gitlab-provider.test.mts (3 assertions)

Full suite (343 files / 5225 tests) passes.

Completes the series: #1255 (commands/) → #1256 (utils/dlx/) →
#1257 (utils/update + utils/command/) → #1258 (env/ + constants/) →
#1259 (test/) → this.

* fix(cli): address Cursor bugbot findings on error messages

Four issues flagged by Cursor bugbot on #1260:

1. (Medium) gitlab-provider.mts: error said 'check GL_TOKEN permissions'
   but the actual env var is GITLAB_TOKEN (as the same file's getGitLabToken
   confirms). Fixed to GITLAB_TOKEN.

2. (Medium) git/operations.mts: error suggested 'set SOCKET_CLI_GIT_PATH
   to point at a specific binary' — that env var is not read anywhere.
   Removed the false suggestion; kept the real fix (install git and put
   it on PATH) with package-manager examples.

3. (Low) terminal/iocraft.mts: '(e as Error).message' evaluates to
   undefined when a non-Error is thrown. Switched to
   'e instanceof Error ? e.message : String(e)' for safe stringification.

4. (Low) gitlab-provider.mts: error said 'after ${retries} retries' but
   the loop runs attempt 1..retries inclusive — retries is the total
   attempt count, not retries beyond the first. Reworded to 'attempts'.
   Matching test assertions updated.

Caught by #1260 bugbot review.

* chore(cli): use joinAnd + getErrorCause helpers in utils/ misc

- basics/vfs-extract.mts: missingTools list now renders as prose
  via joinAnd('a, b, and c').
- terminal/iocraft.mts: inline `e instanceof Error ? e.message :
  String(e)` swapped for getErrorCause(e). require() of a native
  binding can throw non-Error values, so the safe-stringify with
  UNKNOWN_ERROR fallback is correct here.

No behavior change for Error throws.
John-David Dalton (jdalton) added a commit that referenced this pull request Apr 24, 2026
)

* fix(cli): align env/ + constants/ + build-script error messages with 4-ingredient strategy

Rewrites runtime and build-time error messages for the build-inlined
version/checksum pipeline to follow the What / Where / Saw vs. wanted /
Fix strategy from CLAUDE.md.

Sources (runtime):
- env/coana-version.mts, env/sfw-version.mts (2 getters),
  env/socket-basics-version.mts, env/socket-patch-version.mts,
  env/trufflehog-version.mts, env/trivy-version.mts,
  env/opengrep-version.mts, env/pycli-version.mts — 9 "INLINED_X
  not found" errors. Each now names the exact env var, the
  bundle-tools.json path it comes from, and how to rebuild
  (`pnpm run build:cli`).
- env/checksum-utils.mts — parseChecksums() and requireChecksum()
  now show the exact JSON.parse error or the list of known assets
  so you can see what was in vs. out of the map.
- constants/paths.mts — getSocketRegistryPath() now enumerates
  every env var the app-data lookup checks (HOME, USERPROFILE,
  LOCALAPPDATA, XDG_DATA_HOME) so a cold environment tells you
  which to set.

Sources (build-time scripts, same message style for consistency):
- scripts/sea-build-utils/downloads.mts — 3 checksum-missing
  errors in the SEA build path, each now names the bundle-tools.json
  key and tells you to run `pnpm run sync-checksums`.

No tests pinned these messages (only dist/cli.js — unchecked-in
build output).

Follows strategy from #1254. Continues #1255, #1256, #1257.

* chore(cli): harden (e as Error) casts to safe stringify

Switch `(e as Error).message` to `e instanceof Error ? e.message : String(e)` so that when a non-Error value is thrown (strings, objects, null) the error message stays informative instead of becoming 'undefined'.

Same fix as applied to #1260 (iocraft.mts) after Cursor bugbot flagged the pattern on that PR.

* fix(cli): address Cursor bugbot findings on checksum error messages

Two issues flagged by Cursor bugbot on #1258:

1. (Low) parseChecksums() built the env var name as
   `INLINED_${toolName.toUpperCase()}_CHECKSUMS`. When toolName has
   spaces (e.g. 'Socket Patch'), toUpperCase() produces 'SOCKET PATCH'
   → 'INLINED_SOCKET PATCH_CHECKSUMS' — not a valid env var name. The
   real env var is INLINED_SOCKET_PATCH_CHECKSUMS.

2. (Low) Both parseChecksums() and requireChecksum() embedded
   `tools.${toolName}.checksums` to reference bundle-tools.json paths,
   but toolName is the display name (PyCLI, OpenGrep, Socket Patch)
   not the case-sensitive JSON key (socketsecurity, opengrep,
   socket-patch).

Both came from the same root cause: I treated the display-name
parameter as if it were a canonical identifier. Fix: reword the
messages to just name the tool in prose ('inlined checksums for X',
'X has no SHA-256 for Y') and point at the 'matching entry in
bundle-tools.json' instead of inventing a wrong path. Keeps the
4-ingredient structure (what/where/saw/fix) without claiming
identifiers that don't exist.

Caught by #1258 bugbot review.

* chore(cli): use joinAnd from @socketsecurity/lib/arrays for error lists

Switch the 4 `Object.keys(x).join(', ')` calls in error messages on
this branch to `joinAnd(Object.keys(x))` so they render as human
prose (e.g. 'a, b, and c') instead of machine-y comma-joins.

Sites:
- src/env/checksum-utils.mts: requireChecksum known-assets list
- scripts/sea-build-utils/downloads.mts: 3 missing-checksum errors
  (external tools, socketsecurity wheel, socket-basics archive)

No behavior change — just uses the fleet helper consistently.

* fix(cli): use bracket notation for hyphenated tool keys in error message

Cursor flagged the checksum-missing error in downloads.mts: it used
\`tools.\${toolName}.checksums\` (dot notation) which produces an
invalid JSONPath like \`tools.socket-patch.checksums\` when toolName
is hyphenated. The socket-basics site a few hundred lines down already
uses bracket notation for the same reason; make this one match.

Reported on PR #1258.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants